Skip to content

Conversation

@moonglum
Copy link
Member

@moonglum moonglum commented Mar 11, 2022

After experimenting with esbuild in #149, I wanted to try out the other new "transpiler" swc. That's the one used by Deno and Parcel. Note that this PR does not replace rollup, it uses swc for transpilation ('esnext', TypeScript & JSX) and minification. swc bundling is still under development, and I think it is too early for us.

My impression is very positive. The generated code is very similar to what we have right now, but in the places where it is different, I prefer the output to the one from Babel (for example the way it compiles class).

Notes:

  • This introduces one breaking change: The TypeScript code is no longer type checked.
  • Compiling should be significantly faster (Rust over JS, but also: all the steps that were done by different tools are now done in one big step), I'll try to do some benchmarks.
  • The dependencies of our 'extension packages' are now all identical (but you only will install swc if you choose one of the extensions), and we reduce the dependencies drastically.
  • Minification is a bit less efficient, as we minify each file separately, and not the result of the entire thing.
  • We keep rollup-plugin-cleanup for compact=true and compact="compact".
  • The biggest advantage I see compared to esbuild is that it fully supports browserslist, without any hacks.
  • Drops support for the module format cjs, we deprecated it, as we use commonjs instead.

pragma: jsx?.pragma,
pragmaFrag: jsx?.fragment

// TODO: Improve JSX Handling
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want, we can add the 'modern' JSX option (automatic import) here as well. We could also address #144 if we want.

@moonglum moonglum requested a review from FND March 11, 2022 12:11
Copy link
Contributor

@FND FND left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good - though of course it needs serious real-world testing.

The fact that so few changes are required makes me kinda suspicious - though it might just prove that we found the right abstractions? Only time will tell; see above.

plugins = plugins.concat(determineCompacting(compact));
}
if(compact === true || compact === "compact") {
plugins = plugins.concat(require("rollup-plugin-cleanup")());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is leaky now; we don't want to spread this kinda awareness across both config.js and swc.js.

generateSWCConfig might return both the config object and a list of plugins to be added here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, true 🤔 So generateSWCConfig would not only handle SWC, but also plugin-cleanup, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd think so, yes; seems like sensible encapsulation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried it out, but this leads to the following effects:

  • We need to push down the generation of the swc plugin to the generateSWCConfig file
  • The generateSWCConfig needs to check if it should even add the swc plugin

Overall, I don't really like it 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of those seem like advantages to me rather than disadvantages. 🤷

My argument boils down to deletability: If we ever replace SWC again (which, given this very PR, doesn't seem improbable), we'd most likely have to revisit these three lines as well; that doesn't seem obvious to me at all.

Having said that:
I can't be worried about that shit. Life goes on, man.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I thought about it again and found a way that should appease us both. Take a look at the new commits starting from de16c9d

@moonglum moonglum mentioned this pull request Jan 12, 2023
7 tasks
@FND FND force-pushed the v3 branch 2 times, most recently from 655e79c to f6f714d Compare January 16, 2023 10:29
Base automatically changed from v3 to main January 16, 2023 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants